feat(pipeline): Capture Trace flag #2293
Conversation
It is very difficult to capture new data from Clickhouse with the current return value from the function. The current wrapper around the clickhouse-driver library returns a simple list of results, that gets overloaded with column types when that meta information is returned. Change the return type to be an object, so that any data that we might want to propogate up from Clickhouse becomes easy to add without having to go through this process again.
|
So this means that technically any snuba API call can request a trace. Is that what we want? Are there any dangers in exposing this functionality to all callers? |
|
Is there any reason to separate the change where it gets wired through? |
|
Right, the flag needs to be passed via the API request but I'm unsure how to make it so only the admin portal can use it. @onewland This doesn't need to be a separate PR but I'd like to figure out how to limit the exposure of the flag in the API and we can work on using it later. |
Codecov Report
@@ Coverage Diff @@
## master #2293 +/- ##
==========================================
- Coverage 92.96% 92.96% -0.01%
==========================================
Files 556 556
Lines 25436 25447 +11
==========================================
+ Hits 23646 23656 +10
- Misses 1790 1791 +1
Continue to review full report at Codecov.
|
Added capture_trace flag to RequestSettings and schema Updated execution pipeline to propagate flag through to ClickHouse execute() methods
| # Flag to retrieve only ClickHouse tracing data. If this is True, | ||
| # the query execution should not return the results of the query, but | ||
| # return the tracing data of the query execution within ClickHouse | ||
| "capture_trace": {"type": "boolean", "default": False}, |
There was a problem hiding this comment.
I am not sure we want to expose this in the snuba api.
We cannot change the log handler there if that impacts the system logs or snuba logs, which means we would not be using this in that circumstance.
Also this may not work in HTTP and we want to move away from the TCP protocol for the Snuba queries, which may mean this will not be able to work on the snuba api.
I would suggest we remove it from there till we do not decide to expose it.
Overview
RequestSettingsand gets propagated through to query executionChanges
capture_traceflag toRequestSettingsand schemaexecute()methods